590 apps specific - #591
Conversation
github-actions
Bot
added
the
cla-required
Joerg Henrichs (hiker)
added
the
Ready for review
|
Yaswant Pradhan (@yaswant) , Matthew Hambley (@MatthewHambley) , Sam Clarke-Green (@t00sa) - I have no idea why the CLA check failed, as far as I can see I have signed the CONTRIBUTORS file: With Otherwise ready for review. |
Joerg Henrichs (@hiker) I cant see an entry for you in the https://github.com/MetOffice/fab/blob/main/CONTRIBUTORS.md?plain=1 or in your PR branch. If you update that file in your branch the check should pass. If you have signed the CLA in a different PR, maybe that is not merge in main yet so the cla-checker cant see it? Once that PR is merged, the error should disappear. |
|
Yes indeed, I looked at the wrong branch - sorry. I might just add it to all my PRs, to give you flexibility in which order to review them :) Thanks! |
github-actions
Bot
added
cla-signed
mo-lucy-gordon
left a comment
mo-lucy-gordon
left a comment
There was a problem hiding this comment.
I have some small suggested changes for the documentation file but otherwise the documentation looks good and builds correctly.
Is there a plan to change the unit tests imports that fail in later PRs?
I ran the unit test in test_fab_base.py and that produces a bunch of errors but they seem to be from previously added code, so as long as you're happy that they are still there that's fine?
|
|
||
| Application-specific settings | ||
| ============================= | ||
| Besides site-specific settings, the Fab base class also allows to use |
There was a problem hiding this comment.
| Besides site-specific settings, the Fab base class also allows to use | |
| Besides site-specific settings, the Fab base class also allows |
| An example of this is LFRic. The infrastructure (lfric_core) repository | ||
| contains site-specific configuration. For example, they will define | ||
| the required compilation flags for files. These settings will be used | ||
| even for applications in applications in the lfric_apps repository. |
There was a problem hiding this comment.
| even for applications in applications in the lfric_apps repository. | |
| even for applications in the lfric_apps repository. |
| But certain applications needs additional flags. For example, the | ||
| lfric_atm application will compile the UM physics code, and this require | ||
| that by default any real values are double precision (and in some cases | ||
| file-specific work arounds for compiler bugs. To avoid that the site-settings |
There was a problem hiding this comment.
| file-specific work arounds for compiler bugs. To avoid that the site-settings | |
| file-specific work arounds for compiler bugs). To avoid that the site-settings |
| lfric_atm application will compile the UM physics code, and this require | ||
| that by default any real values are double precision (and in some cases | ||
| file-specific work arounds for compiler bugs. To avoid that the site-settings | ||
| from lfric_core need to be duplicated, the following structure is |
There was a problem hiding this comment.
| from lfric_core need to be duplicated, the following structure is | |
| from lfric_core need to be duplicated. The following structure is |
| def __init__(self): | ||
| ... | ||
|
|
||
| This setup will allow to reuse site-specific setup, which can be overwritten |
There was a problem hiding this comment.
| This setup will allow to reuse site-specific setup, which can be overwritten | |
| This setup will allow us to reuse site-specific setup, which can be overwritten |
| into the ``default`` setup. | ||
|
|
||
| .. important:: | ||
| If there is a application-specific configuration, it is important that |
There was a problem hiding this comment.
| If there is a application-specific configuration, it is important that | |
| If there is an application-specific configuration, it is important that |
3 participants
This allows lfric_atm to use lfric-atm-specific compilation option.